feat(orchestration-v2): attribute reused subagents to the run driving them (3/5) - #4662
feat(orchestration-v2): attribute reused subagents to the run driving them (3/5)#4662shivamhwp wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
9296337 to
760a4c2
Compare
5187d5b to
cde2598
Compare
760a4c2 to
f3666e9
Compare
cde2598 to
fc5b293
Compare
f3666e9 to
2565ea0
Compare
85fe8a2 to
795eb20
Compare
… them A subagent row keeps the run id it was spawned under. Routing admits a row by run or by parent thread, and the parent thread is excluded by construction, so a later run re-activating that subagent could only see its updates while the spawning run's ingestion fiber happened to still be alive. The Claude adapter already re-attributed on reopen for exactly this reason; this brings the same treatment to routing and to Codex. Reuse arrives on item/completed, not item/started: resumeAgent and sendInput only ever complete, and registerSubagentThreads ignores them because they are not spawnAgent. Hooking the rebind there is what makes it fire — subagent_continue now asserts the subagent is attributed to the run that drove it, where before it stayed pinned to the spawning run. Routing gains a third test: subagent identity, seeded from the projection at turn start. The child thread is adopted only while the agent is live, because a settled row is also re-emitted by trailing traffic — a token usage frame or a collab state sweep — and adopting on those would hand a later run an interrupted agent's thread, readmitting the stale events that post-interrupt recovery exists to exclude. A subagent's row and its timeline item are emitted together, so both route by identity; matching only the row advanced the agent while its item stayed frozen on the spawning run. The reactivation predicate is separate from canRouteRelatedSubagent rather than a widening of it, for the same reason: an interrupted agent must not have its child thread pre-owned, but its identity is still resumable, because the user stopped it rather than losing it. Rehydration seeds turn ordinals alongside the registry — restarting them at 1 would re-derive childRootNodeId for a different native turn — and parks non-terminal leftovers at idle, since a new session can never terminalize an activation it did not drive and would otherwise keep the session pinned open. Not proven: rehydration itself. The fixture that would exercise it is recorded but does not replay to completion, for reasons unrelated to these changes, and is left unregistered with the open question written up in fixtures/index.ts.
…estart A fresh adapter is the post-restart state: the process-local registry is empty, so a returning native child thread can only be recognised through the identities the projection hands back in existingSubagents. Drives a transcript with no spawn frame, seeds one idle subagent, and asserts the returning thread reuses that identity, continues the activation count and ordinal rather than restarting them, and keeps the usage recorded before the restart. Paired with a negative control that replays the same transcript unseeded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bc22aac to
2567faf
Compare
795eb20 to
64bf642
Compare
There was a problem hiding this comment.
Effect service conventions: one issue found. Known tagged failures should be recovered with Effect.catchTags rather than catchIf + a schema predicate.
Posted via Macroscope — Effect Service Conventions
| Effect.catchIf(Schema.is(ProjectionStoreThreadNotFoundError), () => | ||
| Effect.succeed(null), | ||
| ), |
There was a problem hiding this comment.
catchIf with a schema predicate is being used only to recover the known tagged failure ProjectionStoreThreadNotFoundError. Per the service conventions, statically known tagged failures should be recovered with Effect.catchTags({ ... }) (even for a single tag); catchIf is reserved for genuinely structural predicates. This also matches the pervasive Effect.catchTags usage elsewhere in the codebase.
| Effect.catchIf(Schema.is(ProjectionStoreThreadNotFoundError), () => | |
| Effect.succeed(null), | |
| ), | |
| Effect.catchTags({ | |
| ProjectionStoreThreadNotFoundError: () => Effect.succeed(null), | |
| }), |
Posted via Macroscope — Effect Service Conventions
Co-Authored-By: Claude <noreply@anthropic.com>
32cbb3d to
c63297e
Compare
Third of five stacked PRs replacing #4551. Stacked on #4629 — review that first.
A subagent row keeps the run id it was spawned under. Routing admits a row by run or by parent thread, and the parent thread is excluded by construction — so a later run re-activating that subagent could only see its updates while the spawning run's ingestion fiber happened to still be alive. The Claude adapter already re-attributed on reopen for exactly this reason; this brings the same treatment to routing and to Codex.
The bug that made this invisible
The rebind was hooked to
item/started. Codex's reuse frames —resumeAgent,sendInput— only ever arrive onitem/completed, andregisterSubagentThreadsignores them because they aren'tspawnAgent. So the rebind never fired, and I could not reproduce the fix working.Moving the hook is what makes it observable.
subagent_continuenow asserts the subagent is attributed to the run that drove it:subagent.runIdordinal:1(spawning run)ordinal:2(run that drove the reuse)Routing
Identity becomes a third ownership test, seeded from the projection at turn start. Two constraints shape it:
That is also why the reactivation predicate is separate from
canRouteRelatedSubagentrather than a widening of it: an interrupted agent must not have its child thread pre-owned, but its identity is still resumable — the user stopped it, they didn't lose it.Rehydration
Seeds turn ordinals alongside the registry (restarting at 1 would re-derive
childRootNodeIdfor a different native turn) and parks non-terminal leftovers atidle, since a new session can never terminalize an activation it didn't drive and would otherwise keep the session pinned open.Rehydration is covered directly.
CodexAdapterV2.test.tsclears the process-local registry, seeds the next turn from projection-known subagents, and verifies that the same identity is reused with activation ordinal/count and lifetime usage advanced correctly. The recordedsubagent_reuse_after_idlefixture remains unregistered while its broader replay path is investigated.Testing
Typecheck clean across server, contracts, client-runtime, web. Server 1,684 passing.
Reviewed by an independent agent pass, which found the
item/started/item/completedbug above plus the child-thread adoption hole, the row/item routing asymmetry, and both rehydration gaps. All fixed here.Note
Attribute reused subagents to the run reactivating them in orchestration-v2
CodexAdapterV2by rebuilding the process-local registry fromexistingSubagentsand rebinding them to the active run on collab agent interactions.RunExecutionServiceto match subagent rows, activations, and turn items by identity (not just run/thread), so reused subagents are admitted and tracked correctly.canReactivateSubagentto allow reactivation of idle/interrupted subagents; failed/cancelled are excluded. Classifiesidleas a terminal status for cascade/routing purposes.currentActivationIdviacascadeTerminalizeRunOwnedSubagents.subagent_reuse_after_idlescenario; the fixture is scaffolded but not yet registered in the test suite due to a replay hang.currentActivationIdis null, replacing the removedisSettledCodexSubagentcheck.📊 Macroscope summarized 85fe8a2. 10 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
(Automatic summaries will resume when PR exits draft mode or review begins).🗂️ Filtered Issues
No issues evaluated.